Skip to content

fix: display invalid tag information#1375

Merged
Lee-W merged 1 commit intocommitizen-tools:masterfrom
carlossg:invalidtag
Apr 9, 2025
Merged

fix: display invalid tag information#1375
Lee-W merged 1 commit intocommitizen-tools:masterfrom
carlossg:invalidtag

Conversation

@carlossg
Copy link
Contributor

@carlossg carlossg commented Mar 21, 2025

and the regex used for validation

Description

Starting in 4.4.0 due to #1297 when an invalid tag is present an exception is thrown. I would expect the tag is ignored.

This PR only improves the message, not the root cause

Before

packaging.version.InvalidVersion: Invalid version tag

After

packaging.version.InvalidVersion: Invalid version tag: '2.2.44.aa00000' does not match any configured tag format: re.compile('v?(?P<version>([0-9]+)\\.([0-9]+)(?:\\.([0-9]+))?(?:-([0-9A-Za-z-]+(?:\\.[0-9A-Za-z-]+)*))?(?:\\+[0-9A-Za-z.]+)?(\\w+)?)')

Checklist

  • Add test cases to all the changes you introduce
  • Run poetry all locally to ensure this change passes linter check and test
  • Test the changes on the local machine manually
  • Update the documentation for the changes

Expected behavior

Steps to Test This Pull Request

  1. create a non semver tag, ie. 2.2.44.aa00000
  2. run cz bump --dry-run

Additional context

@carlossg carlossg mentioned this pull request Mar 21, 2025
Merged
4 tasks
noirbizarre
noirbizarre requested changes Mar 21, 2025
View reviewed changes
Copy link
Member

@noirbizarre noirbizarre left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be a good addition, but without the regexps in the message as it will easily flood the output.

Note that #1297 should not have introduced any new raised exception, as it was already raised before and it was the reason I made #1297 initially, to provide a workaround to this exception. So I am also interested in finding why it wasn't raised before for you and now it is.

@carlossg
Copy link
Contributor Author

carlossg commented Mar 24, 2025

I have setup a minimal reproducer in https://github.com/carlossg/commitizen-test

with my patch

❯ cz version; cz bump --dry-run
4.4.1
Traceback (most recent call last):
  File "/opt/homebrew/bin/cz", line 8, in <module>
    sys.exit(main())
             ~~~~^^
  File "/opt/homebrew/Cellar/commitizen/4.4.1_1/libexec/lib/python3.13/site-packages/commitizen/cli.py", line 656, in main
    args.func(conf, arguments)()
    ~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/opt/homebrew/Cellar/commitizen/4.4.1_1/libexec/lib/python3.13/site-packages/commitizen/commands/bump.py", line 147, in __call__
    current_version = self.scheme(provider.get_version())
                                  ~~~~~~~~~~~~~~~~~~~~^^
  File "/opt/homebrew/Cellar/commitizen/4.4.1_1/libexec/lib/python3.13/site-packages/commitizen/providers/scm_provider.py", line 21, in get_version
    versions = sorted(rules.extract_version(t) for t in version_tags)
  File "/opt/homebrew/Cellar/commitizen/4.4.1_1/libexec/lib/python3.13/site-packages/commitizen/providers/scm_provider.py", line 21, in <genexpr>
    versions = sorted(rules.extract_version(t) for t in version_tags)
                      ~~~~~~~~~~~~~~~~~~~~~^^^
  File "/opt/homebrew/Cellar/commitizen/4.4.1_1/libexec/lib/python3.13/site-packages/commitizen/tags.py", line 148, in extract_version
    raise InvalidVersion(f"Invalid version tag: '{tag.name}' does not match any configured tag format: {regexes}")
packaging.version.InvalidVersion: Invalid version tag: '1.0.0.fb3a1d1' does not match any configured tag format: re.compile('v?(?P<version>([0-9]+)\\.([0-9]+)(?:\\.([0-9]+))?(?:-([0-9A-Za-z-]+(?:\\.[0-9A-Za-z-]+)*))?(?:\\+[0-9A-Za-z.]+)?(\\w+)?)')

I can remove the regex to be less verbose

@carlossg @Lee-W
fix: print which tag is invalid
2a0472c 
and the regex used for validation
Lee-W
Lee-W approved these changes Apr 6, 2025
View reviewed changes
Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just fix the style. Would be nice if we could add a unit test. But I'm also good with merge it as it is

@codecov
Copy link

codecov bot commented Apr 6, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.55%. Comparing base (120d514) to head (2a0472c).
Report is 580 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1375      +/-   ##
==========================================
+ Coverage   97.33%   97.55%   +0.21%     
==========================================
  Files          42       57      +15     
  Lines        2104     2655     +551     
==========================================
+ Hits         2048     2590     +542     
- Misses         56       65       +9
Flag Coverage Δ
unittests 97.55% <100.00%> (+0.21%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Lee-W Lee-W changed the title fix: print which tag is invalid fix: display invalid tag information Apr 6, 2025
@Lee-W Lee-W requested a review from noirbizarre April 6, 2025 04:17
noirbizarre
noirbizarre approved these changes Apr 8, 2025
View reviewed changes
Copy link
Member

@noirbizarre noirbizarre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is already a test for the exception itself, so I am OK to have the wording changed without a new test.

I'll take a look at the uncaught exception as soon as I have more time.

@Lee-W
Copy link
Member

Lee-W commented Apr 9, 2025

I think there is already a test for the exception itself, so I am OK to have the wording changed without a new test.

I'll take a look at the uncaught exception as soon as I have more time.

Then that's merge it and we can get back a bit later

@Lee-W Lee-W merged commit d666aa0 into commitizen-tools:master Apr 9, 2025
18 checks passed
@carlossg carlossg deleted the invalidtag branch May 12, 2025 17:05
carlossg added a commit to carlossg/commitizen that referenced this pull request May 12, 2025
@carlossg
fix: ignore invalid tags
c57b3bd 
Starting in 4.4.0 due to commitizen-tools#1297 when an invalid tag is present an exception is thrown

This commit ignores the invalid tags

Follow up from commitizen-tools#1375
@carlossg carlossg mentioned this pull request May 12, 2025
Closed
4 tasks
carlossg added a commit to carlossg/commitizen that referenced this pull request May 12, 2025
@carlossg
fix: ignore invalid tags
ba5e539 
Starting in 4.4.0 due to commitizen-tools#1297 when an invalid tag is present an exception is thrown

This commit ignores the invalid tags

Follow up from commitizen-tools#1375
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@noirbizarre noirbizarre noirbizarre approved these changes

@Lee-W Lee-W Lee-W approved these changes

@woile woile Awaiting requested review from woile woile is a code owner

Assignees

No one assigned

Labels

pr-status: wait-for-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@carlossg @Lee-W @noirbizarre